refactor(downgrader): simplify converters, tests, and fixtures - #7
Merged
Conversation
- Replace the UnknownRecord alias with the inline Record type - Rename HTTP_METHODS to HTTP_METHODS_UP_TO_V31, since 3.2 adds `query` - Drop comments that restate the code; module headers point at the README - Move convertSchema below its field table, removing an eslint-disable - Share dig and expectValidAs across the tests; cast only malformed input - Type fixture exports as OpenAPIObject instead of `satisfies`, so tests use them without casts
…query-method-a991f1
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Converter source cleanup —
UnknownRecordinlined toRecord<string, unknown>throughoutshared.tsand both converters;HTTP_METHODSrenamedHTTP_METHODS_UP_TO_V31;convertSchemamoved below theSCHEMA_FIELDStable so theno-use-before-definesuppression is gone;convertSubschemas/convertContent/convertResponsesarrow-consts became function declarations;convertParameterListinlined at its two call sites. I diffed the converter sources base→head directly: the changes are mechanical (moves, renames, comment deletions), no behavioral drift. - Doc comment trim — the removed comments restated code or duplicated the README; I checked that the README's mapping tables cover every construct the trimmed module headers now defer to.
- Test helper consolidation — the three duplicated copies of
dig/expectValidAs/validatemerged intotests/helpers.ts;as anysurvives only on deliberately malformed inputs (non-objects, missing-required-field docs, 3.2-only fields fed to the 3.1→3.0 hop), matching the PR's claim. - Fixture typing — 82 fixtures flip from
satisfies OpenAPIObjectto: OpenAPIObject, letting the corpus/e2e tests pass them straight into the converters. The tictactoe fix is real and I reproduced it: withsatisfies,security: [{ defaultApiKey: [] }, { app2AppOauth: ['board:read'] }]infers a union whose members get?: undefinednormalized twins, which the[name: string]: string[]index signature rejects — the annotation contextually types the literal and avoids that union. Excess-property checking still applies to the annotated literals, so the fixtures keep rejecting disallowed properties.
Verified locally: pnpm type:check, pnpm lint, and pnpm test (327 tests, including the snapshot-pinned converter outputs and the 72 corpus round-trips) all pass; the corpus fixture lists are unchanged (36 + 36 entries).
DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes (delta since the prior pullfrog review of 1141c4c)
- Merge of
origin/maininto the branch (e984453) — no downgrader or types content changed; the merge bringsmainin cleanly. - Workflow whitespace trim (
f2dcfb5) —.github/workflows/pullfrog.ymldrops trailing spaces from two blank lines. Whitespace only, no behavioral surface. - The other 92 files carry the same diff hashes as the previously reviewed commit; the full refactor stands as reviewed and passes
pnpm type:check,pnpm lint, and all 327 tests.
DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cleanup pass over the downgrader package, prompted by checking that the OpenAPI 3.2
queryoperation is covered (it is, in the types, the converter, and the tests). The converters lose comments that restated the code, the tests lose duplicated helpers and blanket casts, and the shared fixtures become typed exports that tests consume without casts. Conversion behaviour is unchanged.Source
UnknownRecordalias removed in favour of the inline type.HTTP_METHODSis nowHTTP_METHODS_UP_TO_V31, making explicit that 3.2 addsqueryon top of the list.convertSchemasits below its field table, so theno-use-before-definesuppression is gone; the remaining arrow-function consts are function declarations.Tests
digandexpectValidAslive in onetests/helpers.tsinstead of three copies.export const doc: OpenAPIObjectrather thansatisfies, so they keep the real type and pass into the converters directly. This also fixes the tictactoe fixture, whosesecurityunion TypeScript widened into a shape the index signature rejected.as anyremains only on deliberately malformed inputs: junk values, missing required fields, and wrong-version documents.Testing
pnpm lint,pnpm type:check, andpnpm testpass (327 tests).